-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make isAuthenticated in middleware fail more loudly. #144
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
CHANGELOG.md
Outdated
returned false in the case of a Convex backend that didn't expose an endpoint | ||
called `auth:isAuthenticated`, now it throws an error. This should help people | ||
with the migration required for 0.0.76. | ||
|
||
## 0.0.78 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically this is unreleased too, I think - I just speculatively added the next version # when I updated this file earlier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah gotcha
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG, but see other comments.
src/nextjs/server/index.tsx
Outdated
"Server Error: could not find api.auth.isAuthenticated. convex-auth 0.0.76 introduced a new export in convex/auth.ts. Add `isAuthenticated` to the list of functions returned from convexAuth(). See convex-auth changelog for more https://github.com/get-convex/convex-auth/blob/main/CHANGELOG.md", | ||
); | ||
} | ||
throw e; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need to return false here. Otherwise errors like sending an invalid JWT to the Convex backend will also throw (which is why that one test is failing, I think).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hooray for tests!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah for the token expired and invalid token cases, got it. Maybe we can pick those out too, for now I'll log whatever error it is.
commit: |
Make the Next.js middleware function
isAuthenticated
fail more loudly. Previously it returned false in the case of a Convex backend that didn't expose an endpoint calledauth:isAuthenticated
, now it throws an error. This should help people with the migration required for 0.0.76.